Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a single base class for all app types #2244

Closed
wants to merge 12 commits into from

Conversation

freakboy3742
Copy link
Member

@freakboy3742 freakboy3742 commented Nov 26, 2023

[Most of the comments below refer to an older version of this design. To see the differences between the two versions, view the edit of this comment on Mar 18, 2024 – @mhsmith]

Following the discussion on #2238, a revised approach to the window type question.

In this PR:

  • All apps are instance of toga.App.
  • A toga.MainWindow is a "window with full decoration". This means it includes a menu bar (on Windows and GTK), and a titlebar on iOS and Android. On macOS, the app-level menubar still exists, but the list of commands is vastly reduced.
  • The Document API has been significantly modified to allow for the full document lifecycle.
  • Any app can have a list of document types assigned to it. This means that you can have an app that both has a main window and has document-centric "file/open" menu items.
  • If declare document types, and you start the app with command line arguments, every argument matching a known document type opens a document window; if no documents (or no openable documents) are specified, and the platform requires at least one window (i.e., GTK and Windows) an empty document window is provided.
  • The New/Open/Save/Save As/Save All menu items all have default implementations. If you define document types; or, as a user, you define methods that override the default implementations, the corresponding menu items will be added and enabled.
  • The Preferences menu item has a similar approach. If, as a user, you override the preferences menu item, it will be enabled, and your implementation will be invoked. Otherwise, the menu item will exist, but be disabled. This is a placeholder to indicate that we will (eventually) add default preferences handling to Toga apps.
  • The object that is assigned to toga.App.main_window determines the behavior of the app:
    • Assigning a toga.MainWindow instance generates the historical app behavior.
    • Assigning a toga.Window instance generates a "simple" app, with minimalist menus/titlebars
    • Assigning None results in a Session-based app. This is what a "Document-based app" was previously called, but it now covers the generic case of web/file browsers that don't have a "document" per se, but have the same "close on last window"/"persistent app with no windows" behavior.
    • Assigning the constant toga.App.BACKGROUND makes the app a background app - an app with no requirement for windows, and a hidden app icon (where possible). This is the placeholder for a status icon app.
    • Assigning any other type/value, raises an error
    • Assigning no value (i.e. forgetting to set main_window) is also an error.
  • App exit behavior is now dependent on main_window, rather than being specifically bound to toga.MainWindow.
    • If main_window is a Window (or subclass), closing that window closes the app.
    • If main_window is None, the exit behavior is platform dependent; see discussion below.
    • If main_window is toga.App.BACKGROUND, it's up to the user to define a way to exit the app. This isn't very helpful at present, but will make more sense once status icons are added.

Session-based (nee Document-based) apps are now fully implemented on all desktop platforms. On Cocoa, the behavior hasn't changed; the app instance is persistent, and there's an app-level "exit" menu option. On GTK and Winforms, the behavior has been modified so that there's a check when closing windows; when the last window in the app is closed, the app exits.

This PR doesn't implement status icons; status icons become a relatively straightforward piece of app-level menu item handling once this PR is implemented.


The current state of this PR implements tests for the core API, and is fully documented; but the testbed is currently showing coverage gaps. That said, there are very few testbed tests required - most of the new behaviour has been kept in the core.

In addition to an app like tutorial2 that exercises the "MainWindow" case, there are three new examples:

  • simpleapp - The simple "no app menu" case.
  • documentapp - A document-based app, loading text documents as documents.
  • statusiconapp - A skeleton for a status icon app. It has no UI at present; it exists purely to demonstrate that an app can be persistent without having an open window.

However, my intention is that this PR won't even land. It is unmanageably large to review in it's current state - but it is useful as context for some of the design decisions that have been made, and it's a useful proof-of-concept that the final API design works in practice across all platforms. The following is a list of discrete changes that can be factored out of this work; I'll update this list with PR references as they are submitted:

@LunaMeadows
Copy link
Contributor

Running Ubuntu 22.04 with Wayland, the PR works as expected for a document app. Closing all windows also seems to follow system behaviors of other applications.

Only issue that I can see is spacing below menu bar and text field as discussed in the discord.

@HalfWhitt
Copy link
Contributor

My two cents: I think this arrangement feels more intuitive than the multiple-classes route, and quite possibly avoids snarly inheritance problems down the road.

There's a use case this abstraction doesn't quite cover, though. I don't think document-based, create/open/edit-style apps are the only ones that are suited to the behavior you describe of 1) having no main window and 2) remaining persistent (Mac) or closing (Windows/GTK) when the last window is closed. Consider practically every web browser, as well as any filesystem browser (at least third party ones that aren't special-cased by the OS).

I feel like it would be good to have a way to flag an app to say more generally "All my windows are equally important" without binding it specifically to document files. Your proposed system can get the appropriate native behavior for that on macOS by setting main_window = None and opening whatever windows you want, but then it doesn't exit with the last window on Windows and GTK. (And doing so is kind of an antipattern, if None is supposed to indicate the use-case of a status-icon-style app.)

I also think naming the attribute and the not-necessarily-related class "main window" is definitely misleading, but at present I'm not sure what I'd recommend instead.

@freakboy3742
Copy link
Member Author

There's a use case this abstraction doesn't quite cover, though. I don't think document-based, create/open/edit-style apps are the only ones that are suited to the behavior you describe of 1) having no main window and 2) remaining persistent (Mac) or closing (Windows/GTK) when the last window is closed. Consider practically every web browser, as well as any filesystem browser (at least third party ones that aren't special-cased by the OS).

That's a good point. I agree this is a use case we should support. The first thought that comes to mind is to allow main_window to be set to an explicit sentinel object (call it "LAST_WINDOW" for now, but I'd like a better name) that encompasses the "close on last window / persist" behavior. That replaces the "set the Document class" as the behavior that triggers this for Document-based apps; the default document type becomes the first document type in the collection that is registered.

In that way, "browser"-type apps and document apps would both declare themselves as self.main_window = LAST_WINDOW, getting the behavior we currently have for document-based apps.

I also think naming the attribute and the not-necessarily-related class "main window" is definitely misleading, but at present I'm not sure what I'd recommend instead.

Introducing the LAST_WINDOW sentinel might give a path out here - if we can find a name that encompasses the "window that is the exit sentinel" concept. exit_window perhaps?

@HalfWhitt
Copy link
Contributor

Hmm. main_window = LAST_WINDOW or even exit_window = LAST_WINDOW is indeed a nice, clear description for how it's implemented on GTK/Windows, less so for Mac. Ideally one would be specifying the "flavor" or "intent" of app they want to make, and the persistent/close-on-last-window behavior is abstracted away.

If the concept is centrality, i.e. "What is the central window this app presents as", then it does start to make sense for None to somehow apply to both the "browser" and "headless" cases. But what if it's not the main window? What if it's something like central_control (ideally something like that but less silly-sounding)? Then you can set it to None to literally mean "Nothing is the most important" and get the browser behavior, or you can set it to the status icon (or a sentinel like STATUS_ICON, if need be). And if the main item is a window, well then, exit the app when that's closed.

Admittedly even that last one starts to break down slightly when you think about Mac apps like Activity Monitor that have one primary window but also persist, so it's still not a perfectly mapped abstraction.

@freakboy3742
Copy link
Member Author

Hmm. main_window = LAST_WINDOW or even exit_window = LAST_WINDOW is indeed a nice, clear description for how it's implemented on GTK/Windows, less so for Mac.

I agree LAST_WINDOW isn't a great description of what happens on macOS, but naming things is hard. Unless we can come up with a good name for the "lifecycle is tied to window count" concept, I'm comfortable with using a slightly awkward name, and documenting around the problem.

Ideally one would be specifying the "flavor" or "intent" of app they want to make, and the persistent/close-on-last-window behavior is abstracted away.

That's usually spelled "subclassing" :-)

And, FWIW, it doesn't really solve the naming problem, because now we need to find a name for the app subclass that encompasses the same concept.

If the concept is centrality, i.e. "What is the central window this app presents as", then it does start to make sense for None to somehow apply to both the "browser" and "headless" cases. But what if it's not the main window? What if it's something like central_control (ideally something like that but less silly-sounding)? Then you can set it to None to literally mean "Nothing is the most important" and get the browser behavior, or you can set it to the status icon (or a sentinel like STATUS_ICON, if need be). And if the main item is a window, well then, exit the app when that's closed.

Except that you don't "close" a status icon - there's a menu item that exposes "exit".

Admittedly even that last one starts to break down slightly when you think about Mac apps like Activity Monitor that have one primary window but also persist, so it's still not a perfectly mapped abstraction.

FWIW - I'm definitely comfortable leaving some esoteric use cases on the table. Toga isn't the toolkit you're going to use to write every app - a cross platform framework can't be that. There are always platform-specific details that can't be cleanly abstracted.

@HalfWhitt
Copy link
Contributor

Ideally one would be specifying the "flavor" or "intent" of app they want to make, and the persistent/close-on-last-window behavior is abstracted away.

That's usually spelled "subclassing" :-)

Okay, point taken. 😆 But it could also be spelled "having one or more flags/options on a class" like you've proposed here, it just depends what those flags are...

Except that you don't "close" a status icon - there's a menu item that exposes "exit".

That's true, but is it a problem? If it's the concept of main_item, no one's saying that thing has to be "closed". If a window is the main thing, closing it exits the app — because closing is a thing you can do to windows. If the main item is a status icon, then that's the main control point, and you can exit the app from it. If nothing's the main item, then it either persists (Mac) or exits once all its windows are closed (Winfows/GTK).

I dunno. It makes sense to me, at least, but no two brains are alike, after all.

@freakboy3742
Copy link
Member Author

Ideally one would be specifying the "flavor" or "intent" of app they want to make, and the persistent/close-on-last-window behavior is abstracted away.

That's usually spelled "subclassing" :-)

Okay, point taken. 😆 But it could also be spelled "having one or more flags/options on a class" like you've proposed here, it just depends what those flags are...

Oh, absolutely. I was just tickled that the way you were describing it was so close to subclassing without saying the magic word.

More importantly, using flags allows us to have multiple orthogonal behavioural concepts - we can have flags for "this is how the app closes" and "this is how the app displays the app icon", rather than locking in "One type of app is a DocumentApp, which has these specific behaviours" etc.

Except that you don't "close" a status icon - there's a menu item that exposes "exit".

That's true, but is it a problem? If it's the concept of main_item, no one's saying that thing has to be "closed". If a window is the main thing, closing it exits the app — because closing is a thing you can do to windows. If the main item is a status icon, then that's the main control point, and you can exit the app from it. If nothing's the main item, then it either persists (Mac) or exits once all its windows are closed (Winfows/GTK).

I dunno. It makes sense to me, at least, but no two brains are alike, after all.

Hrm... So - I guess what you're advocating for here is that rather than making main_window = Document the special case, and then trying to work out how to allow both Document and Window both enable the "exit/persist on last window close" behavior, we say that apps that don't nominate a main window will fall back to the "exit/persist on last window behavior", and we make the Status Icon use case sentinel case.

That definitely allows us to avoid the problem of trying to find a name for the "exit/persist on last window close" use case. toga.main_window = None means "there is no 'main' window - all windows are equal, but when there are no windows, the app might exit (platform dependent)", and toga.main_window = toga.BACKGROUND_APP becomes a marker of "this app will run even when it has no windows open" (and, on cocoa, hides the app icon).

@HalfWhitt
Copy link
Contributor

Yep, that's exactly what I meant!

main_window value App behavior
toga.BACKGROUND_APP Background app (systray / menubar item)
Any window instance App with a main window
None An app that (presumably) has windows, but no main window

And document apps would just be the last type, with one or more document types associated.

@freakboy3742
Copy link
Member Author

Yep, that's exactly what I meant!

Ok - The one gap this leaves is the "Default window" case. If main_window = None, what happens if the user doesn't actually open a window?

We currently use the main_window=Document case to evaluate whether there might be a need to open a default window. I guess we could pivot that to "if startup doesn't create a window"; but on macOS, it won't (it opens a file dialog; and the app commands which could be used to open a window will be visible). What happens in case of a "bad" Finder-style app that doesn't declare any document type, but also doesn't open a window as part of startup? There's no windows open... but also no trigger to see if the app should exit. Should we just accept this as an odd use case? Should we raise an error if a non-persistent platform doesn't have an open window at the end of the startup process?

@HalfWhitt
Copy link
Contributor

Ah, I had not thought of that.

I could see an argument for either option (accepting the behavior or raising an error), or for a middle ground of printing a warning. Personally I'd lean toward raising an error, so no one inadvertently makes an uncloseable app that their users have to force kill.

@mhsmith
Copy link
Member

mhsmith commented Nov 29, 2023

Sorry for taking so long to respond. I like the way this is going, but I think the main_window property is getting a bit overloaded. Here's an alternative idea:

UI element visibility:

  • We already talked about Per-window commands #2210. If there was a Window.commands property, this could be used to control the menu bar visibility – it would default to an empty CommandSet, but if you want to hide the menu bar, you can set it to None. On macOS, there would be no visible difference between an empty CommandSet and None.

  • Similarly, if you want a status-icon-only app with no menu or dock icon, you can set App.commands = None. I guess this should also block you from creating any windows. But I see from Maccy that it is possible for such an app to show a dialog, though there's no need to implement that right now.

  • If the platform allows hiding the title bar, we can expose that as Window.title = None.

App and window lifecycle:

  • The only difference between MainWindow and Window would be that closing a MainWindow exits the app. But there's no need to enforce that there's exactly one MainWindow. We could say that if you have any MainWindows, then closing the last one will exit the app (subject to the on_exit handler). If you never created any MainWindows, but only plain Windows, then closing them does nothing special.

  • Then, if we remove the requirement for the startup method to assign main_window, that property can simply be removed. Existing code that assigns to it will create a simple attribute with no special behavior.

  • As discussed above, this does open up a couple of ways for a buggy app to continue running despite having no visible UI, but I think that's a price worth paying for flexibility. And by avoiding anything that permanently commits the app to a particular structure, it makes it easier to cover all structures in the testbed.

Documents:

  • If I understand correctly, the only element of the DocumentApp API that requires platform-specific support is the OS opening a document in an existing process. If we exposed that as an on_open event of the App class, could DocumentApp become a platform-independent subclass of App that can be covered entirely by the core unit tests? And if a user wanted to respond to that event without using the Document API, they'd be free to do so.

Not all of this necessarily has to done in a single PR – in fact, it would probably be easier if it wasn't.

@HalfWhitt
Copy link
Contributor

HalfWhitt commented Nov 29, 2023

The only difference between MainWindow and Window would be that closing a MainWindow exits the app. But there's no need to enforce that there's exactly one MainWindow. We could say that if you have any MainWindows, then closing the last one will exit the app (subject to the on_exit handler). If you never created any MainWindows, but only plain Windows, then closing them does nothing special.

How do you envision this handling the platform-distinct handling of "browser"-style apps? If all its windows are MainWindows and you close the last one, the app (correctly) exits on Windows. We would want it to instead keep running on Mac, but we can't just exempt Mac from exiting on the MainWindow being closed because smaller, single-window apps usually should still exit when that window is closed.

(On the other hand, implementing it as all normal windows would get the correct behavior on Mac, but it would keep running on Windows too.)

@freakboy3742
Copy link
Member Author

UI element visibility:

  • We already talked about Per-window commands #2210. If there was a Window.commands property, this could be used to control the menu bar visibility – it would default to an empty CommandSet, but if you want to hide the menu bar, you can set it to None. On macOS, there would be no visible difference between an empty CommandSet and None.

I think I see how this would/could work; some questions about specifics:

  • The current behaviour of a child window is that it doesn't have a menubar. That makes them useful as the starting point of a complex dialog. Are you suggesting that any existing toga.Window instances would need to gain a menubar=None call as part of a migration? The alternative would be to say that Window has no menubar by default, but MainWindow does... but that then complicates the process of adding commands to a Window, as the commandset won't exist by default. That might be fixable by making the property setter more complex... but it's at least flagging that it's a complexity.
  • In the Windows/GTK case where you have more than one window with menus: Is there a use case for a window that does have a menu bar but doesn't have the default app "file/edit/view" menus? I'm not sure if this is actually desirable or not - the current design allows for a child window that doesn't have the app menus, but I'm not sufficiently familiar with Windows/GTK to know whether this is a use case that is HIG compliant.
  • Similarly, if you want a status-icon-only app with no menu or dock icon, you can set App.commands = None. I guess this should also block you from creating any windows. But I see from Maccy that it is possible for such an app to show a dialog, though there's no need to implement that right now.

FWIW, the design in #2238 does allow for status icons to open windows (and the demo app does). When you do, the app-level menus aren't visible (that is tied to the 'hide app icon' behavior), but I believe that could be controlled on a "gain focus" basis. The ongoing discussion on #2238 has flagged some problems and a potential redesign that might happen in this area, but the constraint of possibly using app commands to register status icons is worth keeping in mind.

  • If the platform allows hiding the title bar, we can expose that as Window.title = None.

I'm not sure how practical this is. If nothing else, it gives the impression that a window can change from "has titlebar" to "doesn't have titlebar" arbitrarily... which might be possible to implement, but it strikes me as an axis of flexibility that isn't a great idea.

It's also functionality that I'm not sure we'll be able to implement at all on non-mobile platforms. That's not fundamentally an issue - we can definitely have features that are desktop- or mobile-specific - but if we can avoid them, I'd argue that's preferable.

App and window lifecycle:

  • The only difference between MainWindow and Window would be that closing a MainWindow exits the app. But there's no need to enforce that there's exactly one MainWindow. We could say that if you have any MainWindows, then closing the last one will exit the app (subject to the on_exit handler). If you never created any MainWindows, but only plain Windows, then closing them does nothing special.

There's a discrepancy, which @HalfWhitt has picked up on; but it's not just "browser" style apps, it's also document based apps. In these, closing the last window exits the app on Windows/GTK, but on macOS, we need the app to persist.

  • Then, if we remove the requirement for the startup method to assign main_window, that property can simply be removed. Existing code that assigns to it will create a simple attribute with no special behavior.

Agreed that having one less thing to configure (and potentially cause confusion) is desirable.

However, I think differentiating "this is what the main window looks like" from "this is how the app closes" does make some kind of sense - and provides a way to differentiate the Document/Browser app use case. I will freely admit I don't love main_window as a name for this, but it's a pre-existing name (so there's no migration churn required) and it's isn't fundamentally misleading.

There's also something to be said for the fact that in the simplest use case, the MainWindow and app.main_window do align, so there's a pedagogical path where in the simple case you say "set main_window = MainWindow() to create a main window, and then clarify that definition over time as the user learns.

  • As discussed above, this does open up a couple of ways for a buggy app to continue running despite having no visible UI, but I think that's a price worth paying for flexibility. And by avoiding anything that permanently commits the app to a particular structure, it makes it easier to cover all structures in the testbed.

Agreed that the "buggy app" use case is unfortunate, but not something we need to take extraordinary measures to avoid.

Documents:

  • If I understand correctly, the only element of the DocumentApp API that requires platform-specific support is the OS opening a document in an existing process. If we exposed that as an on_open event of the App class, could DocumentApp become a platform-independent subclass of App that can be covered entirely by the core unit tests? And if a user wanted to respond to that event without using the Document API, they'd be free to do so.

As noted above, the lifecycle of the app itself is also a factor.

Transforming open into an event is an interesting idea; a lot of the mechanics in the current implementation are very close to being an event, but "hard coded". Making open (et al) explicit events also means we've got a clear way to determine why the new/open/save menu items should be displayed - document apps would install default handlers that tie to the document APIs, but non-document apps could define on_open handlers to provide a way to provide file-based behavior without actually becoming full document apps (say, a single-window graphing app that is able to load a data set).

Not all of this necessarily has to done in a single PR – in fact, it would probably be easier if it wasn't.

Agreed. I included document apps here to flesh out whether the core ideas held together, but once we've got a high level design that we know can be implemented, it will likely be easier to revisit this PR in parts.

@mhsmith
Copy link
Member

mhsmith commented Dec 3, 2023

  • The alternative would be to say that Window has no menubar by default, but MainWindow does... but that then complicates the process of adding commands to a Window, as the commandset won't exist by default.

I think that's a reasonable default, and it's no big inconvenience to create a CommandSet, as long as we make it part of the public API. (EDIT: although, if menus were the only difference between MainWindow and Window, then this would be unnecessary – see below.)

  • In the Windows/GTK case where you have more than one window with menus: Is there a use case for a window that does have a menu bar but doesn't have the default app "file/edit/view" menus?

On GTK and Windows, the only default commands are Exit, Preferences (which should probably be removed, because it doesn't have any implementation), and About. I don't think there's any problem with having those in every window that has a menu bar.

  • If the platform allows hiding the title bar, we can expose that as Window.title = None.

I'm not sure how practical this is. If nothing else, it gives the impression that a window can change from "has titlebar" to "doesn't have titlebar" arbitrarily... which might be possible to implement, but it strikes me as an axis of flexibility that isn't a great idea.

It's also functionality that I'm not sure we'll be able to implement at all on non-mobile platforms. That's not fundamentally an issue - we can definitely have features that are desktop- or mobile-specific - but if we can avoid them, I'd argue that's preferable.

Yes, adding or removing a title bar in an existing window might be difficult to implement, so maybe we say that you have to decide at construction time, and you can't switch between None and str thereafter.

Title-bar-less windows are certainly a thing on desktop platforms, but I agree it's not worth implementing them just now. Even so, I think title=None is the most intuitive way of spelling it, and it keeps it independent of everything else we're discussing here.

If all its windows are MainWindows and you close the last one, the app (correctly) exits on Windows. We would want it to instead keep running on Mac, but we can't just exempt Mac from exiting on the MainWindow being closed because smaller, single-window apps usually should still exit when that window is closed.

Good point. In that case, I guess we do need a separate flag, and maybe we can use that to avoid the "undead app" case as well. Imagine an App.auto_exit boolean, which means "exit the app if, at any point after the end of the startup method, it has no windows and no status icons". The exit could still be cancelled by App.on_exit, which would allow it to be covered by the testbed. If an auto-exit happened immediately after the end of startup, we would print a warning.

Auto-exit would default to true, but DocumentApp would set it to false on macOS. This would automatically cover the case where an app has a single window, or multiple equivalent windows, which should cover a large majority of apps. If the developer wants to do something more complex, like exit the app when a specific window closes, or when all of a particular class of windows has closed, it would be easy enough to implement that themselves.

This suggests that we would remove the special on_close behavior of MainWindow, and the only difference between MainWindow and Window would be that MainWindow has a menu.

there's a pedagogical path where in the simple case you say "set main_window = MainWindow() to create a main window, and then clarify that definition over time as the user learns

That would still work with this proposal. main_window would just become a regular attribute with no special meaning to Toga.

@freakboy3742
Copy link
Member Author

  • In the Windows/GTK case where you have more than one window with menus: Is there a use case for a window that does have a menu bar but doesn't have the default app "file/edit/view" menus?

On GTK and Windows, the only default commands are Exit, Preferences (which should probably be removed, because it doesn't have any implementation), and About. I don't think there's any problem with having those in every window that has a menu bar.

Those are the only menu items right now; but that's as much because I'm a lot more familiar with platform expectations on macOS, so I've been able to fill out more menu items. It seems reasonable to expect that this default list will be longer with time. However, that doesn't really change anything - it's really a question of whether not having the core menu set would be more platform appropriate.

Yes, adding or removing a title bar in an existing window might be difficult to implement, so maybe we say that you have to decide at construction time, and you can't switch between None and str thereafter.

Title-bar-less windows are certainly a thing on desktop platforms, but I agree it's not worth implementing them just now. Even so, I think title=None is the most intuitive way of spelling it, and it keeps it independent of everything else we're discussing here.

Except these aren't independent features. In the Android context, you can't hide the titlebar and have menu items. macOS and GTK both put the toolbar in the window title; GTK's preferred menu handling puts the menu in the toolbar as well (see #874). Looking to the future, Android and iOS apps aren't going to be able to use stack-based navigation views (i.e., select item from list, page slides in from the right, slides off to the left when dismissed) if you haven't got a title bar in which to put the navigation elements.

I'm sure we could put in a bunch of gating logic that raises an error if you try to hide the titlebar if the app has commands, if you add a toolbar to an app without a titlebar... or we can make a clear distinction that there's a "simple" window that only has the bare bones necessary to display content, and there's a "full" window that can accomodate all the extra options.

If all its windows are MainWindows and you close the last one, the app (correctly) exits on Windows. We would want it to instead keep running on Mac, but we can't just exempt Mac from exiting on the MainWindow being closed because smaller, single-window apps usually should still exit when that window is closed.

Good point. In that case, I guess we do need a separate flag, and maybe we can use that to avoid the "undead app" case as well. Imagine an App.auto_exit boolean, which means "exit the app if, at any point after the end of the startup method, it has no windows and no status icons". The exit could still be cancelled by App.on_exit, which would allow it to be covered by the testbed. If an auto-exit happened immediately after the end of startup, we would print a warning.

Auto-exit would default to true, but DocumentApp would set it to false on macOS. This would automatically cover the case where an app has a single window, or multiple equivalent windows, which should cover a large majority of apps. If the developer wants to do something more complex, like exit the app when a specific window closes, or when all of a particular class of windows has closed, it would be easy enough to implement that themselves.

Except that this would (a) require the reintroduction of DocumentApp, and (b) results in "boolean but it might be ignored" business logic for auto_exit.

For my money, nominating a "main" window is more explicit, and assigning None or a BACKGROUND constant gives plenty of opportunity to implement platform behavior without contradicting the plain text reading of the value being assigned. Plus "you must assign a main_window, even if it's None" is easy to explain in documentation, and gives the "zombie" safety catch.

That would still work with this proposal. main_window would just become a regular attribute with no special meaning to Toga.

But what would it mean? As I understand what you're proposing, main_window essentially becomes unnecessary from Toga's perspective. It might be useful for an individual app to have what it calls a "main window", but it could just as easily be called "Steve" and it wouldn't alter Toga's operation.

@mhsmith
Copy link
Member

mhsmith commented Jan 14, 2024

it's really a question of whether not having the core menu set would be more platform appropriate.

Including them allows an easy to understand implementation as laid out in #2210. I doubt many developers would have a problem with this, but if they do we can always come back to it later.

Title-bar-less windows are certainly a thing on desktop platforms, but I agree it's not worth implementing them just now. Even so, I think title=None is the most intuitive way of spelling it, and it keeps it independent of everything else we're discussing here.

Except these aren't independent features. In the Android context, you can't hide the titlebar and have menu items. macOS and GTK both put the toolbar in the window title; GTK's preferred menu handling puts the menu in the toolbar as well (see #874).
[...]
we can make a clear distinction that there's a "simple" window that only has the bare bones necessary to display content, and there's a "full" window that can accomodate all the extra options.

On further thought I think you're right. The problem with title=None is that it's inherently platform-specific: we probably won't ever support hiding the title bar on desktop platforms, and if we did, most app developers who wanted it on mobile wouldn't want it on desktop.

So just to clarify, is this the proposal?

  • Window
    • Has a title bar on desktop platforms but not mobile ones
    • Has no menus or toolbar (the existing Window.toolbar property would be moved to MainWindow)
  • MainWindow
    • Has a title bar on all platforms
    • Has a commands property as described in Per-window commands #2210.
    • Has a toolbar, which is visible if and only if it's non-empty.

@mhsmith
Copy link
Member

mhsmith commented Jan 14, 2024

this would (a) require the reintroduction of DocumentApp

I never proposed removing it, I only pointed out that if there was an on_open event, then DocumentApp could be written entirely in terms of public APIs without any other backend-specific support, which would make it much easier for both us and the users to understand.

and (b) results in "boolean but it might be ignored" business logic for auto_exit.

I'm not sure whether this refers to DocumentApp setting it to false on macOS, or the possibility of App.on_exit cancelling the exit, but those both seem easier for the developer to understand than a complex interpretation of the main_window property.

If you're still leaning in that direction, can you specify exactly what that interpretation would be? It's already changed a couple of times during this discussion.

As I understand what you're proposing, main_window essentially becomes unnecessary from Toga's perspective. It might be useful for an individual app to have what it calls a "main window", but it could just as easily be called "Steve" and it wouldn't alter Toga's operation.

Yes, that's correct. The main_window property would no longer exist, and any app code that assigned it would just create a plain user-defined attribute.

@freakboy3742
Copy link
Member Author

So just to clarify, is this the proposal?

Broadly yes. I think there's still some API work to be done on #2210 (specifically, how to differentiate between app-level commands and window-level commands, especially at time of creation); but whatever form that API takes, I think we're agreed on the interpretation - that macOS gets all commands in the single app-level menubar; other platforms get the union of "app level" commands, plus the commands registered against the current window.

I have one other possible clarification/inconsistency:

  • Window
    • Has no menus or toolbar (the existing Window.toolbar property would be moved to MainWindow)

I'm unsure whether these two should be "does not have", or "doesn't not have by default".

Saying that a menu is only available on a MainWindow makes some sense - as you've described, it provides a good entry point for the interpretation of #2210, and allows a way for us to dodge questions of whether we should allow menus without app commands.

However, toolbar is a simpler case, because it doesn't exist at all unless you add something to it. My question is whether there might be a use case for a window that has a toolbar, but no menu. From an implementation point of view, it's not that hard to allow for toolbars on a simple Window (after all, we do right now); the argument against it would be more on the grounds of "conceptual consistency" - that a Window is truly "simple", and a MainWindow is a complex UI element.

I could go either way on this one. Probably the biggest argument in favor of allowing toolbar on Window is that removing it would be backwards incompatible.

this would (a) require the reintroduction of DocumentApp

I never proposed removing it, I only pointed out that if there was an on_open event, then DocumentApp could be written entirely in terms of public APIs without any other backend-specific support, which would make it much easier for both us and the users to understand.

Ok... but the point of this PR was to converge a single App class, rather than needing different specialised app types... so removing it would seem to be the point of the exercise. :-)

That said, I agree we should be trying to converge on having common behavior in the base class; and abstracting on_open behavior so that it has more to do with command-line argument and document_type handling than logic that is implemented on a backend-specific subclass would be preferable. I'm not 100% convinced an event is the right approach, or if it should be closer to the way startup() should be handled, but I agree that we should be aiming to get this logic into the core API of a single base class.

and (b) results in "boolean but it might be ignored" business logic for auto_exit.

I'm not sure whether this refers to DocumentApp setting it to false on macOS, or the possibility of App.on_exit cancelling the exit, but those both seem easier for the developer to understand than a complex interpretation of the main_window property.

I was referring to macOS having a different functional interpretation of "True" of "auto exit". An auto-exit macOS app won't exit. Ok, this could be explained in docs as a platform inconsistency; but I think that representing this in a form that is tied to the high-level role (main_window) rather than the specific interpretation (app exit) makes more sense.

If you're still leaning in that direction, can you specify exactly what that interpretation would be? It's already changed a couple of times during this discussion.

Yeah - apologies for the confusion.

My current understanding is that app.main_window would have 3 possible values:

  • A window instance. When this specific window instance is closed, the app closes.
  • None. App exit is governed by the closing the last window in the app, except on macOS, where the app will persist.
  • A BACKGROUND_APP constant. In this case, on all platforms, the app will persist when there are no open windows.

The advantage I see with this is that it requires the user to be intentional about how the app exits - either naming a window, or explicitly saying that there is no main window. We can use that to catch the case of neglecting to set a main window during startup.

As an independent concern; if an app declares document types, and a command line argument matching that document type is provided, a Document of the registered type will be opened. If an app declares document types, and no command line arguments are provided, the app-specific "default document" behaviour will be triggered - an empty document on Windows and GTK; the "open file" dialog on macOS.

As I understand what you're proposing, main_window essentially becomes unnecessary from Toga's perspective. It might be useful for an individual app to have what it calls a "main window", but it could just as easily be called "Steve" and it wouldn't alter Toga's operation.

Yes, that's correct. The main_window property would no longer exist, and any app code that assigned it would just create a plain user-defined attribute.

I am sympathetic to the idea of removing one usage of the term "MainWindow"; however, in this case, it means App.auto_exit has "fuzzy" behavior (auto exit won't exit on macOS), and we lose the ability to catch the case where the user doesn't declare a window in their startup() method. This seems especially problematic because the default value for auto_exit would need to be True; so the failure mode of "I forgot to define a window" will be an app that appears to not start, or crash as soon as it starts, with no way to report that there's a clearly identifiable code problem.

There's also the case of an app that opens a main window, opens a secondary window (say, a non-modal dialog), and then the user closes the main window. Unless I'm missing something, the auto_exit interpretation would mean that the app stays open because the non-modal dialog is still open.

The alternative is an app-level main window attribute, and a MainWindow class. An introductory app (and, I'd warrant, most apps) will include a app.main_window = toga.MainWindow() call - which is internally consistent; by the point that a user is contemplating assigning different values to main_window, we will be in a position to explain the discrepancies - plus, app.main_window = None reads fairly plainly as 'there is no "main" window in this app'; and it's not that much of a stretch to think of a background app as a "main window is the whole desktop", or similar. Plus, If a user forgets to instantiate a main window, we can catch that case and report it as an error; and in the "modal dialog" case, closing the main window closes the app - which is the behavior you'd actually expect.

@mhsmith
Copy link
Member

mhsmith commented Jan 16, 2024

Window

  • Has a title bar on desktop platforms but not mobile ones
  • Has no menus or toolbar (the existing Window.toolbar property would be moved to MainWindow)

I'm unsure whether these two should be "does not have", or "doesn't not have by default". [...] Probably the biggest argument in favor of allowing toolbar on Window is that removing it would be backwards incompatible.

As you said above, "these aren't independent features. In the Android context, you can't hide the titlebar and have menu items", and you can't have a toolbar either. So I don't see how we can avoid the backward incompatibility here.

My question is whether there might be a use case for a window that has a toolbar, but no menu.

Yes, I think there would be. If we implemented #2210, this could be expressed by setting MainWindow.commands to None.

I was referring to macOS having a different functional interpretation of "True" of "auto exit". An auto-exit macOS app won't exit.

I never said a True value would be ignored on macOS, only that DocumentApp would set it to False on macOS.

Having said that, I think you've convinced me that a simple boolean isn't good enough here. Just a couple of questions about the proposed meaning of main_window:

  • Would BACKGROUND_APP also be the way of disabling the dock icon on macOS?
  • How would closing a window interact with the on_close and on_exit handlers in each of the main_window modes?

Ok... but the point of this PR was to converge a single App class, rather than needing different specialised app types... so removing [DocumentApp] would seem to be the point of the exercise. :-)

I agree with not having SimpleApp / WindowlessApp classes as in #2238. But I think it still makes sense for DocumentApp to be a subclass of App, because it groups together a lot of closely-related elements:

  • The OS's mechanism for requesting an app to open a file
  • File-related menu items
  • The relationship between Document and Window objects

Any app that wants one of these will probably want the others, but an app that doesn't want one of them probably doesn't want any of the others. So rather than putting them all into the base App class, which is quite complex already, I think it would be easier to understand and maintain if we keep it as a separate layer on top, providing a ready-made structure for a specific kind of app. That way, the document-related code and documentation is all in one place, making it easier to get an overview if you want it, and keeping it out of the way if you don't want it.

The other rationale for having a single App class was to make it more testable, but if DocumentApp didn't require any backend-specific code other than the on_open event, then it could be covered entirely by the core unit tests, and the backend tests would only need to cover the on_open event by itself.

@freakboy3742
Copy link
Member Author

As you said above, "these aren't independent features. In the Android context, you can't hide the titlebar and have menu items", and you can't have a toolbar either. So I don't see how we can avoid the backward incompatibility here.

That's a good point - I had forgotten the Android (and potentially iOS) implication here. I guess that's decided that one, then - Window has to be a true "simple" window, without menus or toolbars.

My question is whether there might be a use case for a window that has a toolbar, but no menu.

Yes, I think there would be. If we implemented #2210, this could be expressed by setting MainWindow.commands to None.

Makes sense. In the macOS case, that means the app will only have app commands; the Windows/GTK case could get a little interesting because the default app commands will be hidden from the user - so features like opening help or preferences won't be exposed anywhere. It then falls on the developer to make sure they're exposed as a toolbar item, or via some other interface. However, I think that's firmly in the camp of "well you asked for this, it's up to you to deal with the consequences" :-)

Having said that, I think you've convinced me that a simple boolean isn't good enough here. Just a couple of questions about the proposed meaning of main_window:

  • Would BACKGROUND_APP also be the way of disabling the dock icon on macOS?

That was my thought - but I'm open to other suggestions for ways to designate the dock-hiding behavior.

  • How would closing a window interact with the on_close and on_exit handlers in each of the main_window modes?

We're now in a position to allow on_close on MainWindow (since it's no longer "special" from the perspective of app lifecycle). This is a change, but a backwards compatible one - setting on_close on MainWindow it currently prohibited by API.

On any window, regardless of app mode, on_close controls whether that specific window is allowed to close. If on_close returns false, that prevents the window from closing (and thus any potential for a window-based app exit condition).

If on_close returns True, the behavior depends on the window and the app mode:

  • If the window is the main_window, it triggers the on_exit event, but doesn't close the window. A successful on_exit kills the app (and thus the window); a failed on_exit means the window stays open.
  • If the main_window is None, and this is the last/only window in the app, and the platform is a "close on last window" platform (i.e., windows, GTK), this triggers the on_exit event, with the same semantics as when the window is the main_window. Otherwise (i.e., it's not the last window, or we're on macOS), the window closes, but there's no on_exit event
  • If main_window is BACKGROUND_APP, then the window closes, with no on_exit event.

on_exit can always be triggered, regardless of the main_window mode, even if there are open windows. If it returns True, the app exits, which will close all windows when the process dies.

Side note: we also require platform logic for iOS and Android to raise an error if main_window is None or BACKGROUND_APP, as those modes won't make sense on mobile.

Ok... but the point of this PR was to converge a single App class, rather than needing different specialised app types... so removing [DocumentApp] would seem to be the point of the exercise. :-)

I agree with not having SimpleApp / WindowlessApp classes as in #2238. But I think it still makes sense for DocumentApp to be a subclass of App, because it groups together a lot of closely-related elements:

  • The OS's mechanism for requesting an app to open a file
  • File-related menu items
  • The relationship between Document and Window objects

Any app that wants one of these will probably want the others, but an app that doesn't want one of them probably doesn't want any of the others.

I'm not sure that is true. For example, consider a simple log viewer app - you might want to accept command line arguments to open a specific data source, and have a File/Open menu item to select a new data source, but not have a full multi-window document interface (because you only want the app to manage a single log source).

That's a slightly contrived example, but I don't think it's entirely unreasonable.

So rather than putting them all into the base App class, which is quite complex already, I think it would be easier to understand and maintain if we keep it as a separate layer on top, providing a ready-made structure for a specific kind of app. That way, the document-related code and documentation is all in one place, making it easier to get an overview if you want it, and keeping it out of the way if you don't want it.

Hrm... I think I see what you're saying - I'll have a bit more of a tinker and see what I can make work. I think I can see the vague outline of how document handling can be managed without making the App class excessively complicated (or at least, not having features that overlap with other functionality), but I need to get my thoughts straight.

@mhsmith
Copy link
Member

mhsmith commented Jul 1, 2024

Replaced by a number of smaller PRs listed in the top comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants